Skip to content

Allow adding markers #965

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 15 commits into from
Nov 4, 2025
Merged

Allow adding markers #965

merged 15 commits into from
Nov 4, 2025

Conversation

@gjmooney
Copy link
Collaborator

@gjmooney gjmooney commented Oct 23, 2025 •
edited by github-actions bot
Loading

Description

Checklist

  • PR has a descriptive title and content.
  • PR description contains references to any issues the PR resolves, e.g. Resolves #XXX.
  • PR has one of the labels: documentation, bug, enhancement, feature, maintenance
  • Checks are passing.
    Failing lint checks can be resolved with:
    • pre-commit run --all-files
    • jlpm run lint

📚 Documentation preview: https://jupytergis--965.org.readthedocs.build/en/965/
💡 JupyterLite preview: https://jupytergis--965.org.readthedocs.build/en/965/lite

@github-actions
Copy link
Contributor

github-actions bot commented Oct 23, 2025

Binder 👈 Launch a Binder on branch gjmooney/jupytergis/allow_adding_markers

@gjmooney gjmooney force-pushed the allow_adding_markers branch from f769482 to 14aa790 Compare October 24, 2025 09:41
@gjmooney gjmooney added the enhancement New feature or request label Oct 24, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Oct 24, 2025 •
edited
Loading

Integration tests report: appsharing.space

@gjmooney
Copy link
Collaborator Author

gjmooney commented Oct 24, 2025

bot please update snapshots

@gjmooney gjmooney closed this Oct 24, 2025
@gjmooney gjmooney reopened this Oct 24, 2025
@gjmooney gjmooney marked this pull request as ready for review October 30, 2025 11:55
@mfisher87
Copy link
Member

mfisher87 commented Oct 31, 2025

I won't have time to look closely at this until after JupyterCon. Are markers being saved as a special case of layer so users can export them?

@gjmooney
Copy link
Collaborator Author

gjmooney commented Nov 3, 2025

I won't have time to look closely at this until after JupyterCon. Are markers being saved as a special case of layer so users can export them?

Yea, they're their own type of layer

martinRenou
martinRenou approved these changes Nov 3, 2025
View reviewed changes
Copy link
Member

@martinRenou martinRenou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat! Thanks!

@martinRenou
Copy link
Member

martinRenou commented Nov 3, 2025

@gjmooney the ui-tests failure is legit, the test for filters does not know anymore which button to click:

    Error: locator.click: Error: strict mode violation: getByRole('button', { name: 'Add' }) resolved to 2 elements:
        1) <button class="jp-Dialog-button jp-mod-accept jp-mod-styled jp-Button">Add</button> aka getByRole('button', { name: 'Add', exact: true })
        2) <button value="" part="control" aria-pressed="false" aria-disabled="false" aria-label="Add Marker" class="control icon-only">…</button> aka getByRole('button', { name: 'Add Marker' })

We need to make the tests more explicit

@martinRenou martinRenou merged commit 13ecfc0 into geojupyter:main Nov 4, 2025
14 checks passed
@gjmooney gjmooney deleted the allow_adding_markers branch November 4, 2025 11:00
nakul-py pushed a commit to nakul-py/jupytergis that referenced this pull request Nov 5, 2025
@gjmooney @github-actions @nakul-py
Allow adding markers (geojupyter#965)
9a2fa9a 
* Add add landmark option

* Use a type for modes

* Use const for identify cursor class

* Rename tool

* Implement add landmark option

* Rename landmark to marker

* Add Marker Source schema and new layer/source handling

* Temp icon

* Dont set style when creating marker

* Tidy up

* Create generic mode toggle method

* Use a marker icon

* Add marker source to sourceType

* Update Playwright Snapshots

* Add data to filter button for tests

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@nakul-py
Copy link
Contributor

nakul-py commented Nov 11, 2025 •
edited
Loading

@gjmooney and @martinRenou Can i add this marker icon? This one looks more suitable.

@gjmooney
Copy link
Collaborator Author

gjmooney commented Nov 11, 2025

@gjmooney and @martinRenou Can i add this marker icon? This one looks more suitable.

Sure, go for it.

@nakul-py nakul-py mentioned this pull request Nov 11, 2025
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@martinRenou martinRenou martinRenou approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gjmooney @mfisher87 @martinRenou @nakul-py